refactor(job-queue-plugin): Use multiple BullMQ queues instead of one #3108
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
There is currently one major problem in the core implementation: Every job needs to have a unique ID. But with BullMQ the IDs are only unique within the sequence of a queue. This is the core issue that still needs to be resolved. Edit: This has been solved by using a combination of the queue name and the BullMQ job ID as a unique ID |
|
@michaelbromley we are running into an issue, that it is not possible to display a list of all jobs from all queues in all states (aka the default list view). This is because of limitations of the BullMQ library and the way how the Redis keys are structured. I see two options here:
|
Signed-off-by: Emmanuel Ferdman <[email protected]>
added await to db query causing the value to be promise and causing server crash Issue#2868
Fixes #3217. The sanitization that was introduced to fix a local file traversal attack was overly-aggressive when using s3 and caused it to break in certain cases.
Based on commit 8545267
This property was moved onto the InspectableJobQueueStrategy interface, since it is tightly bound to a specific strategy, and should not be settable from elsewhere, like with regular VendureConfig options.
|
I have read the CLA Document and I hereby sign the CLA 8 out of 9 committers have signed the CLA. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // For S3 storage, we don't need to sanitize the path because | ||
| // directory traversal attacks are not possible, and modifying the | ||
| // path in this way can s3 files to be not found. | ||
| return path.normalize(decodedPath).replace(/(\.\.[\/\\])+/, ''); |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 21 days ago
To fix the problem, we need to ensure that all instances of ../ or ..\ are removed from the input path. One effective way to achieve this is to repeatedly apply the regular expression replacement until no more replacements can be performed. This ensures that all occurrences of the targeted pattern are removed.
We will modify the sanitizeFilePath method to repeatedly apply the regular expression replacement until the input path no longer contains any ../ or ..\ sequences.
| @@ -264,3 +264,7 @@ | ||
| // path in this way can s3 files to be not found. | ||
| return path.normalize(decodedPath).replace(/(\.\.[\/\\])+/, ''); | ||
| let sanitizedPath = path.normalize(decodedPath); | ||
| while (sanitizedPath.includes('..')) { | ||
| sanitizedPath = sanitizedPath.replace(/(\.\.[\/\\])+/, ''); | ||
| } | ||
| return sanitizedPath; | ||
| } else { |
|
I got sucked down a deep rabbithole on this today. I think I'm pretty close to done, but this is much more subtly complex than I first thought. I established some e2e tests, but things are still buggy in manual testing and I don't have time to finish this now for v3.1. |
|
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
Description
Refactors the
bullmq-job-queue-strategyto use a separate BullMQ queue for every Vendure job queue. This should improve the stability and issues with horizontally scaled worker instances. The current implementation pushes all jobs to a single BullMQ queue calledvendure-job-queue, which is not best practice.Resolves #2419 and #1726
Breaking changes
There is no breaking change.
Checklist
📌 Always:
👍 Most of the time: